Skip to content

add sar_trace_sync#368

Open
rudkoLA wants to merge 4 commits into
p2sr:masterfrom
rudkoLA:feat/trace-offset
Open

add sar_trace_sync#368
rudkoLA wants to merge 4 commits into
p2sr:masterfrom
rudkoLA:feat/trace-offset

Conversation

@rudkoLA

@rudkoLA rudkoLA commented Jun 17, 2026

Copy link
Copy Markdown

this is for comparing two traces of tases so you look at a place where they intersect and using the command make the ticks of both tases at the point the same by subtracting some amount of ticks from the slower tas making the bbox be forwarder and aligned with the faster tas
didn't test linux

Comment thread src/Features/PlayerTrace.cpp Outdated
trace_ticks[h.trace_name] = tick;
}

auto fastest_trace = playerTrace->GetTrace(fastest_trace_name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variables fastest_trace_name, fastest_trace

@rudkoLA rudkoLA marked this pull request as draft June 17, 2026 19:49
@rudkoLA rudkoLA marked this pull request as ready for review June 17, 2026 21:11

@Krzyhau Krzyhau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separately handled state for offset sounds weird to me. It should be a part of trace data that gets reset/removed along with the trace itself. In most cases, when you're doing comparisons, the original trace is recorded once.

Additionally, since you're adding offset anyway, it might be useful to leave a command for explicitly defining it for various purposes. sar_trace_offset [trace_name] [offset]. This would also allow you to keep offset of the trace after re-recording if you wish to constantly re-record comparison traces with offsets for whatever reason.

Comment on lines +73 to +74
// Universal map for trace tick offsets to be able to preserve them between trace recordings
std::unordered_map<std::string, int> tickOffsets;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've created extra trace information which is implicitly not cleared when removing traces. I'm not a fan of that and I'd like to know what justifies this choice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make trace offset not get reset if you rerecorded a trace so I needed to store the offsets somewhere that isn't inside the trace itself

}
}

CON_COMMAND(sar_trace_sync, "sar_trace_sync - syncs all the hovered traces to the fastest trace.\n") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we don't want to compare to the trace that's fastest? What if our other traces we're comparing to are faster instead?

I think this feature would be more versatile if you could specify which trace to sync to (that is, sar_trace_sync_all_to [trace_name])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already implemented this but I guess I forgot to push it

@rudkoLA

rudkoLA commented Jun 18, 2026

Copy link
Copy Markdown
Author

Additionally, since you're adding offset anyway, it might be useful to leave a command for explicitly defining it for various purposes. sar_trace_offset [trace_name] [offset]. This would also allow you to keep offset of the trace after re-recording if you wish to constantly re-record comparison traces with offsets for whatever reason.

this was the first thing I implemented but changed it to sync cause i felt I wouldn't need to manually set offset and would just always use sync. also I couldn't find a way to be able to both set offsets and see them I was happy with, the thing I had just used auto complete to show the trace name and offset

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants